-
Notifications
You must be signed in to change notification settings - Fork 12k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc++] Remove get_temporary_buffer
/return_temporary_buffer
#100914
Conversation
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesWorks towards P0619R4/#99985.
Escape hatches:
Drive-by changes:
Full diff: https://github.com/llvm/llvm-project/pull/100914.diff 21 Files Affected:
diff --git a/libcxx/docs/Status/Cxx17.rst b/libcxx/docs/Status/Cxx17.rst
index ad4f8576f03db..ac07d299d7e74 100644
--- a/libcxx/docs/Status/Cxx17.rst
+++ b/libcxx/docs/Status/Cxx17.rst
@@ -40,6 +40,7 @@ Paper Status
.. note::
+ .. [#note-N4259] N4259: ``std::uncaught_exception`` is deprecated since version 20.0.
.. [#note-P0067] P0067: ``std::(to|from)_chars`` for integrals has been available since version 7.0. ``std::to_chars`` for ``float`` and ``double`` since version 14.0 ``std::to_chars`` for ``long double`` uses the implementation for ``double``.
.. [#note-P0226] P0226: Progress is tracked `here <https://https://libcxx.llvm.org/Status/SpecialMath.html>`_.
.. [#note-P0607] P0607: The parts of P0607 that are not done are the ``<regex>`` bits.
diff --git a/libcxx/docs/Status/Cxx17Papers.csv b/libcxx/docs/Status/Cxx17Papers.csv
index 6c657d51f5c7e..f9a87b2cbfd22 100644
--- a/libcxx/docs/Status/Cxx17Papers.csv
+++ b/libcxx/docs/Status/Cxx17Papers.csv
@@ -4,7 +4,7 @@
"`N4169 <https://wg21.link/n4169>`__","LWG","A proposal to add invoke function template","Urbana","|Complete|","3.7"
"`N4190 <https://wg21.link/n4190>`__","LWG","Removing auto_ptr, random_shuffle(), And Old <functional> Stuff.","Urbana","|Complete|","15.0"
"`N4258 <https://wg21.link/n4258>`__","LWG","Cleaning-up noexcept in the Library.","Urbana","|In Progress|","3.7"
-"`N4259 <https://wg21.link/n4259>`__","CWG","Wording for std::uncaught_exceptions","Urbana","|Complete|","3.7"
+"`N4259 <https://wg21.link/n4259>`__","CWG","Wording for std::uncaught_exceptions","Urbana","|Complete| [#note-N4259]_","3.7"
"`N4277 <https://wg21.link/n4277>`__","LWG","TriviallyCopyable ``reference_wrapper``\ .","Urbana","|Complete|","3.2"
"`N4279 <https://wg21.link/n4279>`__","LWG","Improved insertion interface for unique-key maps.","Urbana","|Complete|","3.7"
"`N4280 <https://wg21.link/n4280>`__","LWG","Non-member size() and more","Urbana","|Complete|","3.6"
diff --git a/libcxx/docs/Status/Cxx20.rst b/libcxx/docs/Status/Cxx20.rst
index b76e30fbb3712..a13496bd0a8fd 100644
--- a/libcxx/docs/Status/Cxx20.rst
+++ b/libcxx/docs/Status/Cxx20.rst
@@ -44,7 +44,7 @@ Paper Status
.. [#note-P0645] P0645: The paper is implemented but still marked as an incomplete feature
(the feature-test macro is not set).
.. [#note-P0966] P0966: It was previously erroneously marked as complete in version 8.0. See `bug 45368 <https://llvm.org/PR45368>`__.
- .. [#note-P0619] P0619: Only sections D.8, D.9, D.10 and D.13 are implemented. Sections D.4, D.7, D.11, and D.12 remain undone.
+ .. [#note-P0619] P0619: Section D.4 remains undone.
.. [#note-P0883.1] P0883: shared_ptr and floating-point changes weren't applied as they themselves aren't implemented yet.
.. [#note-P0883.2] P0883: ``ATOMIC_FLAG_INIT`` was marked deprecated in version 14.0, but was undeprecated with the implementation of LWG3659 in version 15.0.
.. [#note-P0660] P0660: The paper is implemented but the features are experimental and can be enabled via ``-fexperimental-library``.
diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst
index df08875c13bea..8937639bd2bf2 100644
--- a/libcxx/docs/UsingLibcxx.rst
+++ b/libcxx/docs/UsingLibcxx.rst
@@ -215,6 +215,9 @@ C++17 Specific Configuration Macros
C++20 Specific Configuration Macros
-----------------------------------
+**_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION**:
+ This macro is used to re-enable `uncaught_exception`.
+
**_LIBCPP_ENABLE_CXX20_REMOVED_SHARED_PTR_UNIQUE**:
This macro is used to re-enable the function
``std::shared_ptr<...>::unique()``.
@@ -231,6 +234,9 @@ C++20 Specific Configuration Macros
**_LIBCPP_ENABLE_CXX20_REMOVED_RAW_STORAGE_ITERATOR**:
This macro is used to re-enable `raw_storage_iterator`.
+**_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER**:
+ This macro is used to re-enable `get_temporary_buffer` and `return_temporary_buffer`.
+
**_LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS**:
This macro is used to re-enable `is_literal_type`, `is_literal_type_v`,
`result_of` and `result_of_t`.
diff --git a/libcxx/include/__algorithm/inplace_merge.h b/libcxx/include/__algorithm/inplace_merge.h
index a6bcc66a2fa47..4c6ab527c8188 100644
--- a/libcxx/include/__algorithm/inplace_merge.h
+++ b/libcxx/include/__algorithm/inplace_merge.h
@@ -211,10 +211,7 @@ _LIBCPP_HIDE_FROM_ABI void __inplace_merge(
difference_type __len1 = _IterOps<_AlgPolicy>::distance(__first, __middle);
difference_type __len2 = _IterOps<_AlgPolicy>::distance(__middle, __last);
difference_type __buf_size = std::min(__len1, __len2);
- // TODO: Remove the use of std::get_temporary_buffer
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
- pair<value_type*, ptrdiff_t> __buf = std::get_temporary_buffer<value_type>(__buf_size);
- _LIBCPP_SUPPRESS_DEPRECATED_POP
+ pair<value_type*, ptrdiff_t> __buf = std::__get_temporary_buffer<value_type>(__buf_size);
unique_ptr<value_type, __return_temporary_buffer> __h(__buf.first);
return std::__inplace_merge<_AlgPolicy>(
std::move(__first), std::move(__middle), std::move(__last), __comp, __len1, __len2, __buf.first, __buf.second);
diff --git a/libcxx/include/__algorithm/stable_partition.h b/libcxx/include/__algorithm/stable_partition.h
index 8bb1eaf2d2249..4f09b87ae0f9a 100644
--- a/libcxx/include/__algorithm/stable_partition.h
+++ b/libcxx/include/__algorithm/stable_partition.h
@@ -135,10 +135,7 @@ __stable_partition_impl(_ForwardIterator __first, _ForwardIterator __last, _Pred
pair<value_type*, ptrdiff_t> __p(0, 0);
unique_ptr<value_type, __return_temporary_buffer> __h;
if (__len >= __alloc_limit) {
- // TODO: Remove the use of std::get_temporary_buffer
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
- __p = std::get_temporary_buffer<value_type>(__len);
- _LIBCPP_SUPPRESS_DEPRECATED_POP
+ __p = std::__get_temporary_buffer<value_type>(__len);
__h.reset(__p.first);
}
return std::__stable_partition_impl<_AlgPolicy, _Predicate&>(
@@ -275,10 +272,7 @@ _LIBCPP_HIDE_FROM_ABI _BidirectionalIterator __stable_partition_impl(
pair<value_type*, ptrdiff_t> __p(0, 0);
unique_ptr<value_type, __return_temporary_buffer> __h;
if (__len >= __alloc_limit) {
- // TODO: Remove the use of std::get_temporary_buffer
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
- __p = std::get_temporary_buffer<value_type>(__len);
- _LIBCPP_SUPPRESS_DEPRECATED_POP
+ __p = std::__get_temporary_buffer<value_type>(__len);
__h.reset(__p.first);
}
return std::__stable_partition_impl<_AlgPolicy, _Predicate&>(
diff --git a/libcxx/include/__algorithm/stable_sort.h b/libcxx/include/__algorithm/stable_sort.h
index 726e7e16b3564..69fd85cc798bd 100644
--- a/libcxx/include/__algorithm/stable_sort.h
+++ b/libcxx/include/__algorithm/stable_sort.h
@@ -244,10 +244,7 @@ __stable_sort_impl(_RandomAccessIterator __first, _RandomAccessIterator __last,
pair<value_type*, ptrdiff_t> __buf(0, 0);
unique_ptr<value_type, __return_temporary_buffer> __h;
if (__len > static_cast<difference_type>(__stable_sort_switch<value_type>::value)) {
- // TODO: Remove the use of std::get_temporary_buffer
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
- __buf = std::get_temporary_buffer<value_type>(__len);
- _LIBCPP_SUPPRESS_DEPRECATED_POP
+ __buf = std::__get_temporary_buffer<value_type>(__len);
__h.reset(__buf.first);
}
diff --git a/libcxx/include/__exception/operations.h b/libcxx/include/__exception/operations.h
index 0a9c7a7c7f0d8..4a0a697c00e6e 100644
--- a/libcxx/include/__exception/operations.h
+++ b/libcxx/include/__exception/operations.h
@@ -29,7 +29,9 @@ using terminate_handler = void (*)();
_LIBCPP_EXPORTED_FROM_ABI terminate_handler set_terminate(terminate_handler) _NOEXCEPT;
_LIBCPP_EXPORTED_FROM_ABI terminate_handler get_terminate() _NOEXCEPT;
-_LIBCPP_EXPORTED_FROM_ABI bool uncaught_exception() _NOEXCEPT;
+#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION)
+_LIBCPP_EXPORTED_FROM_ABI _LIBCPP_DEPRECATED_IN_CXX17 bool uncaught_exception() _NOEXCEPT;
+#endif // _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION)
_LIBCPP_EXPORTED_FROM_ABI int uncaught_exceptions() _NOEXCEPT;
class _LIBCPP_EXPORTED_FROM_ABI exception_ptr;
diff --git a/libcxx/include/__memory/temporary_buffer.h b/libcxx/include/__memory/temporary_buffer.h
index 88799ca95c1f3..cb44006954fa7 100644
--- a/libcxx/include/__memory/temporary_buffer.h
+++ b/libcxx/include/__memory/temporary_buffer.h
@@ -22,8 +22,8 @@
_LIBCPP_BEGIN_NAMESPACE_STD
template <class _Tp>
-_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI _LIBCPP_DEPRECATED_IN_CXX17 pair<_Tp*, ptrdiff_t>
-get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
+_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI pair<_Tp*, ptrdiff_t>
+__get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
pair<_Tp*, ptrdiff_t> __r(0, 0);
const ptrdiff_t __m =
(~ptrdiff_t(0) ^ ptrdiff_t(ptrdiff_t(1) << (sizeof(ptrdiff_t) * __CHAR_BIT__ - 1))) / sizeof(_Tp);
@@ -56,20 +56,28 @@ get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
return __r;
}
-template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_IN_CXX17 void return_temporary_buffer(_Tp* __p) _NOEXCEPT {
- std::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
-}
-
struct __return_temporary_buffer {
- _LIBCPP_SUPPRESS_DEPRECATED_PUSH
template <class _Tp>
_LIBCPP_HIDE_FROM_ABI void operator()(_Tp* __p) const {
- std::return_temporary_buffer(__p);
+ std::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
}
- _LIBCPP_SUPPRESS_DEPRECATED_POP
};
+#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER)
+
+template <class _Tp>
+_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI _LIBCPP_DEPRECATED_IN_CXX17 pair<_Tp*, ptrdiff_t>
+get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT {
+ return std::__get_temporary_buffer(n);
+}
+
+template <class _Tp>
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_IN_CXX17 void return_temporary_buffer(_Tp* __p) _NOEXCEPT {
+ std::__libcpp_deallocate_unsized((void*)__p, _LIBCPP_ALIGNOF(_Tp));
+}
+
+#endif // _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER)
+
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___MEMORY_TEMPORARY_BUFFER_H
diff --git a/libcxx/include/__ostream/basic_ostream.h b/libcxx/include/__ostream/basic_ostream.h
index 178359d681567..e0698ccb4842a 100644
--- a/libcxx/include/__ostream/basic_ostream.h
+++ b/libcxx/include/__ostream/basic_ostream.h
@@ -152,7 +152,7 @@ basic_ostream<_CharT, _Traits>::sentry::sentry(basic_ostream<_CharT, _Traits>& _
template <class _CharT, class _Traits>
basic_ostream<_CharT, _Traits>::sentry::~sentry() {
- if (__os_.rdbuf() && __os_.good() && (__os_.flags() & ios_base::unitbuf) && !uncaught_exception()) {
+ if (__os_.rdbuf() && __os_.good() && (__os_.flags() & ios_base::unitbuf) && uncaught_exceptions() == 0) {
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
try {
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
diff --git a/libcxx/include/exception b/libcxx/include/exception
index 5eff8e3f8a4bf..64463e02cb16a 100644
--- a/libcxx/include/exception
+++ b/libcxx/include/exception
@@ -47,7 +47,7 @@ terminate_handler set_terminate(terminate_handler f ) noexcept;
terminate_handler get_terminate() noexcept;
[[noreturn]] void terminate() noexcept;
-bool uncaught_exception() noexcept;
+bool uncaught_exception() noexcept; // deprecated in C++17, removed in C++20
int uncaught_exceptions() noexcept; // C++17
typedef unspecified exception_ptr;
diff --git a/libcxx/include/memory b/libcxx/include/memory
index b940a32c3ebe6..db3386cca4800 100644
--- a/libcxx/include/memory
+++ b/libcxx/include/memory
@@ -182,8 +182,8 @@ public:
raw_storage_iterator operator++(int);
};
-template <class T> pair<T*,ptrdiff_t> get_temporary_buffer(ptrdiff_t n) noexcept;
-template <class T> void return_temporary_buffer(T* p) noexcept;
+template <class T> pair<T*,ptrdiff_t> get_temporary_buffer(ptrdiff_t n) noexcept; // deprecated in C++17, removed in C++20
+template <class T> void return_temporary_buffer(T* p) noexcept; // deprecated in C++17, removed in C++20
template <class T> T* addressof(T& r) noexcept;
template <class T> T* addressof(const T&& r) noexcept = delete;
diff --git a/libcxx/include/syncstream b/libcxx/include/syncstream
index e6f35b6f428ed..9da2abb252bfd 100644
--- a/libcxx/include/syncstream
+++ b/libcxx/include/syncstream
@@ -358,7 +358,7 @@ private:
// TODO Use a more generic buffer.
// That buffer should be light with almost no additional headers. Then
// it can be use here, the __retarget_buffer, and place that use
- // the now deprecated get_temporary_buffer
+ // the now removed get_temporary_buffer
basic_string<_CharT, _Traits, _Allocator> __str_;
bool __emit_on_sync_{false};
diff --git a/libcxx/modules/std/exception.inc b/libcxx/modules/std/exception.inc
index 8e802c7065e0f..0c110e6e743fb 100644
--- a/libcxx/modules/std/exception.inc
+++ b/libcxx/modules/std/exception.inc
@@ -21,6 +21,5 @@ export namespace std {
using std::terminate;
using std::terminate_handler;
using std::throw_with_nested;
- using std::uncaught_exception;
using std::uncaught_exceptions;
} // namespace std
diff --git a/libcxx/src/exception.cpp b/libcxx/src/exception.cpp
index ddb186bf8048f..ac6324cd9fe35 100644
--- a/libcxx/src/exception.cpp
+++ b/libcxx/src/exception.cpp
@@ -6,6 +6,9 @@
//
//===----------------------------------------------------------------------===//
+#define _LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION
+#define _LIBCPP_DISABLE_DEPRECATION_WARNINGS
+
#include <exception>
#include <new>
#include <typeinfo>
diff --git a/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp
index 646569e3d573a..6410c84e926aa 100644
--- a/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/memory.nodiscard.verify.cpp
@@ -10,6 +10,7 @@
// check that <memory> functions are marked [[nodiscard]]
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
// clang-format off
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
index 85d12d08c9675..5fc66339d5f80 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
@@ -282,9 +282,6 @@ test()
assert(array[9] == P(0, 2));
}
#if TEST_STD_VER >= 11 && !defined(TEST_HAS_NO_EXCEPTIONS)
- // TODO: Re-enable this test once we're no longer using get_temporary_buffer().
- // For now it trips up GCC due to the use of always_inline.
-#if 0
{ // check that the algorithm still works when no memory is available
std::vector<int> vec(150, 3);
vec[5] = 6;
@@ -300,7 +297,6 @@ test()
assert(std::is_partitioned(vec.begin(), vec.end(), [](int i) { return i < 5; }));
getGlobalMemCounter()->reset();
}
-#endif
#endif // TEST_STD_VER >= 11 && !defined(TEST_HAS_NO_EXCEPTIONS)
}
diff --git a/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp b/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp
index e368ce177b317..6fa4cb22070c8 100644
--- a/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp
+++ b/libcxx/test/std/language.support/support.exception/uncaught/uncaught_exception.pass.cpp
@@ -7,6 +7,10 @@
//===----------------------------------------------------------------------===//
// UNSUPPORTED: no-exceptions
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
+
// test uncaught_exception
#include <exception>
diff --git a/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp b/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp
index 8499478d1fb38..4c66370fac922 100644
--- a/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp
+++ b/libcxx/test/std/utilities/memory/temporary.buffer/overaligned.pass.cpp
@@ -8,6 +8,7 @@
// UNSUPPORTED: c++03
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
// <memory>
diff --git a/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp b/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp
index 6c0fbf2568fff..5f7fc4571906c 100644
--- a/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp
+++ b/libcxx/test/std/utilities/memory/temporary.buffer/temporary_buffer.pass.cpp
@@ -8,6 +8,7 @@
// <memory>
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER
// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
// template <class T>
diff --git a/libcxx/utils/libcxx/test/modules.py b/libcxx/utils/libcxx/test/modules.py
index b7758dc9a41ee..ed0e0d684327b 100644
--- a/libcxx/utils/libcxx/test/modules.py
+++ b/libcxx/utils/libcxx/test/modules.py
@@ -52,13 +52,6 @@
"std::operator==",
]
-# TODO MODULES remove zombie names
-# https://libcxx.llvm.org/Status/Cxx20.html#note-p0619
-SkipDeclarations["memory"] = [
- "std::return_temporary_buffer",
- "std::get_temporary_buffer",
-]
-
# include/__type_traits/is_swappable.h
SkipDeclarations["type_traits"] = [
"std::swap",
|
bf4e6d5
to
da6d71e
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
Works towards P0619R4. - `std::uncaught_exception` was not previously deprecated. This patch deprecates it since C++17 as per N4259. `std::uncaught_exceptions` is used instead as libc++ unconditionally provides this function. - `std::get_temporary_buffer` is replaced with the internal version `__get_temporary_buffer`. - `std::return_temporary_buffer` is replaced with direct `__libcpp_deallocate_unsized` call. Escape hatches: - `_LIBCPP_ENABLE_CXX20_REMOVED_UNCAUGHT_EXCEPTION` restores `std::uncaught_exception`. - `_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER` restores `std::get_temporary_buffer` and `std::return_temporary_buffer`.
da6d71e
to
f7ff07e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like to see this patch split in 2 patches. One for each removal. That makes reviewing the patch a lot easier.
# Conflicts: # libcxx/docs/Status/Cxx17Papers.csv
get_temporary_buffer
/return_temporary_buffer
Somewhat related: https://reviews.llvm.org/D152208 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really well written patch. I like the approach of using a __scoped_temporary_buffer
. I have a few nitpick comments but this LGTM with those fixed.
# Conflicts: # libcxx/docs/ReleaseNotes/20.rst # libcxx/docs/Status/Cxx20.rst
#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) | ||
if (__is_overaligned_for_new(_LIBCPP_ALIGNOF(_Tp))) { | ||
align_val_t __al = align_val_t(_LIBCPP_ALIGNOF(_Tp)); | ||
__ptr_ = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), __al, nothrow)); | ||
} else { | ||
__ptr_ = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), nothrow)); | ||
} | ||
#else | ||
if (__is_overaligned_for_new(_LIBCPP_ALIGNOF(_Tp))) { | ||
// Since aligned operator new is unavailable, constructs an empty buffer rather than one with invalid alignment. | ||
return; | ||
} | ||
|
||
__ptr_ = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), nothrow)); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this the same logic __libcpp_operator_new
implements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I'd be in favour of copy-pasting the implementation of get_temporary_buffer()
(what was done here) and then refactoring it, to avoid any unintended fallout from this patch.
return; | ||
} | ||
|
||
std::__libcpp_deallocate_unsized((void*)__ptr_, _LIBCPP_ALIGNOF(_Tp)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why unsized when we have the count?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just copied from the old code. I think we can refactor this quite a bit, but I would tackle that as a follow-up patch.
# Conflicts: # libcxx/docs/Status/Cxx20.rst
- C++03 `> >` - Module exporting - `unique_ptr` construction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot more than the previous version. Let's see what Louis thinks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I agree this is simpler now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simplify a few things, but this is an improvement as-is (and it may be better to simplify things separately). LGTM.
align_val_t __al = align_val_t(_LIBCPP_ALIGNOF(_Tp)); | ||
__ptr = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), __al, nothrow)); | ||
} else { | ||
__ptr = static_cast<_Tp*>(::operator new(__count * sizeof(_Tp), nothrow)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this breaks CFI builds. It allocates memory with uninitialized objects and casts them to _Tp*
. I think you're supposed to keep this void*
until you placement-new into it, at which point it becomes an actual object.
The old get_temporary_buffer
had this problem too, but it's marked _LIBCPP_NO_CFI
while this new function isn't. (get_temporary_buffer
on the other hand probably doesn't have to be marked _LIBCPP_NO_CFI
anymore.)
A simple fix would be to mark this new function here _LIBCPP_NO_CFI
.
(Maybe the whole design is a bit questionable though? But I mostly care about CFI no longer working.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now opening a PR to add _LIBCPP_NO_CFI
to this function. Is there a better way to address this problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a void*
for the uninitialized tail and a _Tp*
for the initialized head, I suppose? Not sure :)
Thanks for the quick PR!
…#109682) Follows up #100914. Addresses the issue revealed in #100914 (comment).
…llvm#109682) Follows up llvm#100914. Addresses the issue revealed in llvm#100914 (comment).
…llvm#109682) Follows up llvm#100914. Addresses the issue revealed in llvm#100914 (comment).
Works towards P0619R4/#99985.
The use of
std::get_temporary_buffer
andstd::return_temporary_buffer
are replaced withunique_ptr
-based RAII buffer holder.Escape hatches:
_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER
restoresstd::get_temporary_buffer
andstd::return_temporary_buffer
.Drive-by changes:
<syncstream>
, states thatget_temporary_buffer
is now removed, because<syncstream>
is added in C++20.